Repository navigation
Replace libjpeg-turbo with a JPEG codec in safe Rust - #47074
Open
dylan-conway wants to merge 147 commits into
Open
dylan-conway wants to merge 147 commits into
dylan-conway wants to merge 147 commits into
Conversation
Deletes the dependency, its patches and the allowlist entries of its AVX2 kernels. The build now fails to link: codec_jpeg.rs still calls 14 tj3* functions. The next commits replace them. Co-authored-by: dylanc <dylanc@anthropic.com>
Reads what Bun could read through TurboJPEG: baseline, extended sequential and progressive streams, Huffman or arithmetic coded, lossless Huffman streams of up to 8 bits, at 1/8 to 8/8 of the size, to RGBA or CMYK. Not wired up yet, so the build still fails to link. Co-authored-by: dylanc <dylanc@anthropic.com>
RGBA to 4:2:0 YCbCr, baseline with the standard Huffman tables or progressive with optimal ones, with an optional ICC profile. The output is byte for byte what tj3Compress8 produced. Co-authored-by: dylanc <dylanc@anthropic.com>
Bun links again. codec_jpeg.rs keeps its policy (the pixel limit, which M/8 scale a resize target picks, CMYK conversion, when the ICC profile is kept) and loses the FFI. Co-authored-by: dylanc <dylanc@anthropic.com>
To be removed before this is merged. Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
Collaborator
|
Updated 11:46 PM PT - Nov 7th, 2026
✅ @claude, your commit 1e3c2a36fa6f6e0dce0eb1a3307bd04e8f8fd89a passed in 🧪 To try this PR locally: bunx bun-pr 47074That installs a local version of the PR into your bun-47074 --bun |
Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
The inputs of the tests to come. Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
…n libjpeg-turbo Co-authored-by: dylanc <dylanc@anthropic.com>
metadata() no longer copies it. APP2 segments are borrowed from the input and those after the first scan, which nothing read, are skipped. Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
…rust-17fiip Co-authored-by: dylanc <dylanc@anthropic.com>
Marker codes, the DCT's and the colour conversion's fixed-point numbers and fallible allocation were each written more than once. Names instead of bare bools and pairs where a lint asked for them. Co-authored-by: dylanc <dylanc@anthropic.com>
A transform of one dimension is now scalar code for one column, run on every column by a loop that is alone in its function. Before, most of the 8x8 inverse DCT came out as scalar code. Blocks with only a first row or only low frequencies skip the sums of zeros. 3000x2000, -Ctarget-cpu=nehalem: decoding q80 48 to 32 ms, at 1/2 27 to 19 ms, encoding q80 72 to 54 ms. Co-authored-by: dylanc <dylanc@anthropic.com>
A block keeps its coefficients in row-major order, with a bit for each in zigzag order that says whether it is zero. The Huffman coders walk the bits instead of the 63 coefficients, which a progressive file goes through twenty times. The colour conversion takes pixels apart by shifts, which vectorizes. 3000x2000, -Ctarget-cpu=nehalem: q80 54 to 32 ms, progressive 217 to 111 ms. Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
3000x2000 q80 at 7/8: 73 to 46 ms, at 6/8: 55 to 39 ms, at 5/8: 41 to 34 ms. Co-authored-by: dylanc <dylanc@anthropic.com>
…r-ed with their values Co-authored-by: dylanc <dylanc@anthropic.com>
…y a local Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
Co-authored-by: dylanc <dylanc@anthropic.com>
… node:http tests `objects-closed-through-others.ts` failed on macOS x64 in CI on this branch. It and two programs like it, `objects-closed-in-other-ways.ts` and `frames-of-stores-closed-objects-refuse.ts`, fail on macOS on other branches too. The server drops the connection after the exchange that the test breaks on purpose. On a busy machine the client sends the next request on that connection before it notices, so `fetch` rejects, and one of the two runs that are compared prints a `TypeError` more than the other. A request that fails before the handler ran is now sent again, up to three times. A request that the handler did see is judged as before. The three files are as in #47316, byte for byte, so that they merge without a conflict.
`frozen-sealed-and-closed-objects.ts` and `frozen-sealed-and-closed-heirs.ts` ask the same kind of server in the same way, so they can fail as the three of the commit before did. Neither has been seen to. The first is as in #47316, byte for byte. The second is not changed there. Its counter is called `resent`, because the file has a function `attempt`.
`test/aot/build-scaling/build-scaling-2.test.ts` failed all four attempts on macOS arm64 in CI on this branch: "twice as many functions cost 2.59 times the peakBytes; allowed: 2.5", and 2.61 at one attempt. The same compiled code measured 2.42 on another machine four hours earlier. The growth came with #47281 (mimalloc 3.6.0), which the merge of `main` brought in. The growth is now in the file of budgets, which makes the bound 2.84. And `AOT_BUDGETS` keeps a growth that the machine that writes the file does not judge. The two files are as in #47329, byte for byte, so that they merge without a conflict.
dylan-conway
pushed a commit
that referenced
this pull request
Oct 30, 2026
…r in eleven types `test/aot/engine/ir-6.test.ts` fails on `main` since 73a2043 (#47087), on Alpine, on Windows and in the ASAN build. There the engine prints `builtin(Object)|builtin(FunctionPrototype)|builtin(ObjectPrototype)|..` where this fixture expects `builtin(Object)|builtin(ObjectPrototype)|..`. The eleven lines with that type get a `{{*}}` after `builtin(Object)|`. This is the change that #47074 makes to the file, byte for byte. With it, that PR's build passes on all those lanes. It has nothing to do with the rest of this branch, which touches neither the AOT engine nor its tests.
… got through Bun was built with each of 22 wrong versions of the rule (`start_progressive_scan` in decode/phuff.rs and `start_input_pass` in decode/output.rs) and run against image-adversarial, jpeg-golden and image.test.ts. Four passed all three: without `ah == 0 ||`, so that a second first scan of a coefficient gets through if it goes down to a lower bit; looking at the first coefficient of the band only; and looking at the first, or at the last, component of a scan only, which nothing could tell because the progressive cases have one component. Three more scripts of scans, the test with three components for progressive frames too (Huffman and arithmetic), and two more cases in it, with the component that comes again first and last in its scan. All 22 fail now, each of the four on new cases only. Bun 1.4.4, which decodes such scans, fails every new case.
…t nothing noticed A sweep with kinds of changes that the earlier ones did not make went over src/jpeg and codec_jpeg.rs: one side of a `||` or `&&` dropped, a condition made false, a loop or an `any` over the first or the last item only or over all but it, a `return Err` removed, an alternative of a pattern dropped. 886 changes. 14 of them change what a file decodes to and still passed the golden files and every stream that image-adversarial.test.ts and image.test.ts build: - marker.rs: a scan that names a component twice is found only if it is the first or not the last of those before it (2); a piece of an ICC profile may have a number above the count - output.rs: sampling factors that do not divide the largest are refused in one direction only (2); of a row group of two rows, h2v2 and h1v2 upsampling do the first or the last only (4); the rows around an iMCU row are kept only if a component other than the last needs them - phuff.rs: a correction bit moves a coefficient that has the bit already - coef.rs: whether smoothing is of use is decided without coefficient 1, or by it alone (2) - mod.rs: chroma of 1x2 or 2x1 counts as sampling that TurboJPEG has a name for whatever the first component has Eleven files of 31 to 229 bytes tell them apart. What is expected of them comes from Bun 1.4.4, and this branch gives the same. Bun built with each of the 14 fails the golden test on its new file and on no other file.
…rust-17fiip main replaced libspng with bun_png (#47070) in the places where this branch replaces libjpeg-turbo with bun_jpeg: the lists of dependencies, Cargo.toml, the tables of licenses, the image docs and types, codecs.rs and three test files. Both libraries are gone from the lists, and both crates and both files of Highway kernels stay. Everything under test/aot is main's. #47316 wrote custom-accessor-reached-computed-key.ir anew without the unions that depend on ids, so the wildcards are not needed, and it has the retry in the node:http programs. The recorded growth of one shape of build-scaling is dropped, because main recorded that shape's budget again.
…e one too high Of 77 small changes to src/jpeg that a larger set of inputs had caught only late, the tests in the repository catch 60: all 39 in the encoder by the 466 pictures, and 21 in the decoder, 16 of those by nothing but the damaged copies of the golden files. 15 change only which error a file fails with, which Bun does not show. Two are left: - M_RST7 as 0xd8: an FF D8 where a restart marker is due is dropped, and the file decodes where it has to fail - M_APP15 as 0xf0: an FF F0 segment is skipped like an APPn segment, and the header is read where it has to fail A file of 125 and one of 129 bytes tell them apart. What is expected of them comes from Bun 1.4.4, and this branch gives the same. Bun built with either change fails the golden test on its new file and on no other file.
dylan-conway
added a commit
that referenced
this pull request
Nov 3, 2026
… package manager (#47414) ### What does this PR do? 50 separate changes, one commit each: 16 in the core crates (`bun_core`, `bun_collections`, the TOML and XML parsers, the crash handler), 16 in the resolver and the code next to it (the HTTP client's setup, source maps, the standalone module graph), and 18 in the package manager. Two more commits answer the `mordant` lint job: 51 takes back a part of change 5, and 52 moves a `match` that change 33 left in two places into a method. Nothing is left of commits 53 and 54: they changed three test files so that two of `main`'s tests pass, #47316 has since rewritten those files on `main`, and the merge of `main` takes `main`'s versions. So this PR changes nothing outside `src/`. Commit 56 answers a review comment: `read_directory` and its siblings return a shared reference. The changes follow #47291, #47317, #47338, #47382 and #47393 and work the same way: signatures name the type and the lifetime that the value already has, hand-written pointer code becomes plain ownership and borrows that the compiler checks, and what was never needed is deleted. Together: **126 fewer `unsafe` blocks and 8 fewer `unsafe fn`s**, +877/−1651 over 73 files. No safe function became `unsafe`, and no wrapper type was added. **[Where the compiled code changes](#where-the-compiled-code-changes) lists every place where the program does different work.** | # | Change | `unsafe` | Diff | |---|---|---|---| | 1 | toml: the parser keeps tables and arrays as `StoreRef` | −6 blocks | +43/−64, 1 file | | 2 | bun_core: remove `boxed_zeroed_unchecked`, `ZStr::boxed` and `OptionsEnvArg for Box<ZStr>` | −3 blocks, −1 `unsafe fn` | +3/−50, 4 files | | 3 | xml: `Unit::from_u16` and `Unit::from_bytes` instead of casts | −3 blocks | +20/−22, 1 file | | 4 | crash_handler: `UNSUPPORTED_UV_FUNCTION` holds the name's bytes | −2 blocks | +6/−13, 1 file | | 5 | xml_index: `StructuralIndex::seek` and `at` read the window with `get` | −2 blocks | +14/−10, 1 file | | 6 | collections: `HiveSlot` borrows its hive | −2 blocks, −1 `unsafe fn` | +28/−60, 1 file | | 7 | bun_core: `vec::extend_from_fn` is `Vec::extend` | −1 block | +5/−28, 2 files | | 8 | fmt: the shared temp buffer is an `Option<Box>` | −3 blocks | +65/−119, 1 file | | 9 | SmolStr: `Inlined::all_chars` views the `u128` with `bytes_of_mut` | −1 block | +2/−11, 1 file | | 10 | bun_core: `impl io::Write for Writer` calls the inherent methods | −2 blocks | +2/−6, 1 file | | 11 | atomic_cell: `unreachable!()` for the widths that `Atom` excludes | −1 block | +2/−3, 1 file | | 12 | highway: `memmem` subtracts addresses | −1 block | +2/−2, 1 file | | 13 | Check constant strings at compile time instead of `from_utf8_unchecked` | −2 blocks | +17/−40, 4 files | | 14 | Call `gettid`, `getpid`, `alarm`, `_exit` and `raise` as `safe fn` | −7 blocks, +3 `unsafe extern` blocks | +25/−24, 5 files | | 15 | bun_core: `fd_path_raw_w` takes `&mut [u16]` | −5 blocks, −1 `unsafe fn` | +17/−30, 2 files | | 16 | output: `STDOUT_STREAM` and `STDERR_STREAM` are atomics | −3 blocks | +21/−13, 1 file | | 17 | resolver: `BrowserMapPath::map` is `&'static BrowserMap` | −4 blocks | +5/−12, 1 file | | 18 | resolver: keep the slot that `MaybeUninit::write` returns | −2 blocks | +4/−8, 1 file | | 19 | resolver: drop three lifetime casts that change nothing | −3 blocks | +7/−14, 2 files | | 20 | resolver: `RealFS::read_directory` and `EntryLookup` keep their `'static` | −3 blocks | +21/−55, 4 files | | 21 | http: `h3::ClientContext::get_or_create` reads the loop itself | −2 blocks | +6/−14, 2 files | | 22 | boringssl: `ssl_ctx_setup` takes `&OwnedSslCtx` | −2 blocks, −1 `unsafe fn` | +11/−22, 2 files | | 23 | sourcemap: `impl_pod!` for `SyncEntry` and `ObjectBrace` | −3 blocks | +9/−20, 1 file | | 24 | sourcemap: `WindowReader<'a>` borrows the stream | −1 block | +32/−40, 1 file | | 25 | resolver: four `unsafe` blocks that plain borrows replace | −4 blocks | +18/−38, 4 files | | 26 | resolver: `package_json` travels as `&'static PackageJSON` | −3 blocks | +30/−77, 6 files | | 27 | resolver: `load_tsconfig` keeps its boxes | −4 blocks | +16/−37, 1 file | | 28 | resolver: `BIN_FOLDERS` is initialised at compile time | −2 blocks | +17/−39, 2 files | | 29 | standalone_graph: in `inject`, `zname` borrows `zname_owned` | −2 blocks | +3/−12, 1 file | | 30 | sourcemap: `ParsedSourceMap` holds the standalone graph's sources by type | −1 block | +22/−62, 4 files | | 31 | http: `InitOpts::abs_ca_file_name` is a `&'static ZStr` | −1 block | +14/−36, 3 files | | 32 | exe_format: `_mh_execute_header` is a `safe static` | −1 block | +3/−3, 1 file | | 33 | install: `init` reborrows the `&'static mut DirEntry` it matched on | −2 blocks | +2/−9, 1 file | | 34 | install: `save_lockfile` uses `progress.root` instead of a stashed node pointer | −2 blocks | +2/−15, 1 file | | 35 | install: drop four raw-pointer and lifetime detours that borrowck does not need | −4 blocks | +16/−44, 3 files | | 36 | install: `find_workspace_root_above` borrows `walk.path` for the source path | −1 block | +2/−9, 1 file | | 37 | install: `AutoInstaller::path_for_resolution` takes `&mut PathBuffer` | −1 block | +3/−15, 3 files | | 38 | install: `NetworkTask` reads its manager through the `ParentRef` it stores | −1 block | +7/−22, 1 file | | 39 | install: remove three one-line `unsafe` blocks that have a safe spelling | −3 blocks | +10/−19, 3 files | | 40 | install: clear `root_lifecycle_scripts` with a `scopeguard` on `manager` | −1 block | +3/−10, 1 file | | 41 | semver: delete `query::Group.input` | −1 block | +22/−37, 3 files | | 42 | install: `UpdateRequest.e_string` keeps the `StoreRef` it is made from | −5 blocks | +16/−41, 3 files | | 43 | install: `TarballStream::init` returns a `Box` and `populate_result` is a safe fn | −4 blocks, −2 `unsafe fn` | +149/−202, 4 files | | 44 | install: the Windows hardlink task is passed and reclaimed as a `Box` | −5 blocks, −1 `unsafe fn` | +22/−32, 1 file | | 45 | install: the Windows `Hardlinker` keeps the cwd slice it gets | −1 block | +4/−25, 1 file | | 46 | install: `ROOT_PACKAGE_JSON_PATH` becomes `PackageManager::root_package_json_path` | −8 blocks | +40/−36, 8 files | | 47 | install: `Diff::generate` takes `to_update: bool` instead of the update requests | −1 block | +24/−28, 4 files | | 48 | install: `Package::parse_from_real_manager` takes `&mut PackageManager` | −1 block, −1 `unsafe fn` | +13/−35, 2 files | | 49 | install: in `init`, `original_cwd` borrows the boxed copy of the cwd | −1 block | +3/−9, 1 file | | 50 | install: remove two pieces of dead code that needed `unsafe` | −2 blocks | +29/−61, 2 files | | 51 | xml_index: `seek_refill` calls `at` for every index, as it did | 0 | +4/−8, 1 file | | 52 | resolver: `EntriesOption::try_entries_mut` returns the listing or the read error | 0 | +14/−8, 2 files | | 53 | test: `custom-accessor-reached-computed-key.ir` allows one more member in eleven types | 0 | nothing, since the merge | | 54 | test: record the growth in memory of one shape of build-scaling, as #47329 does | 0 | nothing, since the merge | | 55 | the merge of `main` | | | | 56 | resolver: `read_directory` and its siblings return `&'static EntriesOption` | 0 | +21/−21, 5 files | The commits can be reviewed one at a time. Worth knowing before reading on: - **Nothing that users can see changes**, so the PR adds no test. One leak ends, on a path that hardly ever runs: a `tsconfig.json` chain of more than 64 `extends` frees the configs it has read (change 27). - **Five changes end an aliasing or type bug that had no visible effect.** - Change 30: the standalone graph stored a pointer to one struct as an integer, and the reader cast it to another `repr(Rust)` struct with the same fields. - Change 40: a guard wrote through a raw pointer to the `PackageManager` that was stale by the time it ran. - Change 47: with `--frozen-lockfile` and for `bun pm`, `parse_with_json` made a `&Lockfile` while a `&mut` to the same lockfile was live. - Change 49: `original_cwd` pointed into a buffer that was overwritten while it was still in use. Nothing read the overwritten bytes. - Change 20: a `&mut Entry` could overlap a `&'static Entry`. The store it was made for wrote the value that was already there, so the store is gone. - **Change 14 adds three `unsafe extern "C"` blocks.** They hold `safe fn` declarations of `gettid`, `alarm`, `_exit` and `raise`, next to their callers, as the neighbouring code does for other libc functions. - **New functions, all without `unsafe`:** `Unit::from_u16` and `Unit::from_bytes` (change 3), `with_shared_temp_buffer` (8), `WindowReader::new` (24), the `const fn BoundedArray::new` (28) and `EntriesOption::try_entries_mut` (52). - **Seven changes are compiled only for Windows or macOS, in whole or in part:** 15, 44 and 45 (Windows), 32 (macOS), and single lines of 16, 29 and 43. - **Three structs get smaller, and no file format changes:** `semver::query::Group`, and with it every `Dependency`, by 16 bytes (41), `resolver::Result` and `MatchResult` by 8 (26), and `UpdateRequest` by 8 (42). All of them hold pointers and exist only in memory. - `bun.lockb` stores a `Dependency` as the 26 bytes of `dependency::to_external`: name, name hash, behavior, tag and the literal of the version. The range is not stored. `to_dependency` parses it from the literal again on load. - `write_array` and `read_array` take only `NoUninit` and `AnyBitPattern` types, which `Dependency` and `Group` are not. - `padding_checker.rs` and `bun.lockb.rs` assert at compile time the size and alignment of the structs that are written as they are in memory: `Resolution`, `Meta`, `Bin`, `Scripts`, `PatchedDep`, `semver::Version`, `String` and `ExternalString` among others, and `NpmPackage` and `PackageVersion` of the manifest cache. This PR touches neither those files nor those types. ### Where the compiled code changes I compared the release assembly (x86-64 Linux, `--emit=asm`) of 21 crates, before and after, function by function, with names demangled and labels normalised: the 18 crates that this PR touches, and `bun_jsc`, `bun_io` and `bun_shell_parser`, which use what changes in `bun_core`, `bun_collections` and the resolver. | Crate | Functions | Differ | Other constants or names only | Same length | Shorter | Longer | Instructions | Branches | Compares | Calls | Panic call sites | |---|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:|--:| | `bun_collections`, `bun_highway`, `bun_paths`, `bun_exe_format`, `bun_lint`, `bun_io` | 4,265 | 0 | | | | | | | | | | | `bun_core` | 1,148 | 13 | 1 | 2 | 9 | 1 | −237 | −32 | −34 | −20 | −3 | | `bun_parsers` | 412 | 5 | 1 | 0 | 0 | 4 | +30 | +4 | +4 | +2 | +2 | | `bun_semver` | 70 | 12 | 6 | 2 | 4 | 0 | −18 | 0 | 0 | 0 | 0 | | `bun_crash_handler` | 120 | 2 | 0 | 0 | 2 | 0 | −8 | −2 | −1 | −2 | 0 | | `bun_boringssl` | 30 | 1 | 0 | 1 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | | `bun_sourcemap` | 104 | 9 | 0 | 4 | 1 | 4 | +14 | 0 | 0 | 0 | 0 | | `bun_install_types` | 34 | 5 | 5 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | 0 | | `bun_resolver` | 443 | 29 | 10 | 4 | 13 | 2 | −93 | −2 | −2 | +5 | −1 | | `bun_http` | 380 | 5 | 0 | 1 | 1 | 3 | 0 | 0 | 0 | +1 | +1 | | `bun_standalone_graph` | 208 | 3 | 0 | 0 | 2 | 1 | −12 | 0 | 0 | −1 | 0 | | `bun_install` | 2,819 | 667 | 561 | 45 | 34 | 27 | −165 | −39 | −50 | −16 | −13 | | `bun_bundler` | 1,983 | 25 | 12 | 2 | 5 | 6 | 0 | 0 | −1 | 0 | 0 | | `bun_shell_parser` | 121 | 1 | 0 | 0 | 0 | 1 | +2 | 0 | 0 | 0 | 0 | | `bun_jsc` | 1,707 | 10 | 6 | 1 | 3 | 0 | −6 | 0 | 0 | 0 | 0 | | `bun_runtime` | 10,459 | 98 | 78 | 4 | 4 | 12 | +72 | +10 | +9 | +6 | +2 | The table is from before the merge of `main` and before commit 56. Commit 56 changes no machine code: `.text`, `.rodata`, `.data`, `.data.rel.ro` and `.eh_frame` of a release build for x86-64 Linux are byte for byte the same with and without it. The last five columns are net, and count the functions that exist on one side only: 21 only before and 19 only after, which were removed, renamed or inlined, or are generic code with another type argument. **Most of what differs is the size of four structs.** `semver::query::Group` loses 16 bytes (change 41), so a `Dependency` goes from 240 to 224. `UpdateRequest` goes from 280 to 256 (41 and 42). `resolver::Result` goes from 296 to 288, and `MatchResult` loses 8 too (26). `PackageManager` gains the 16 bytes of its new field (46), which moves the fields behind it. That gives other offsets and strides (the fourth column), and where a stride changes, the compiler also multiplies differently (`imul $296` is now `lea` and `shl`) and shapes loops differently. Of the 106 functions of `bun_install` that differ in more than constants, 90 differ by at most 5 instructions and have no compare, branch or panic site more. So do all 13 of `bun_bundler` and all 4 of `bun_jsc`. **Checks.** One path makes a check that it did not make before: `seek_refill` of the XML index, once per refill of its window, which takes in 8,192 units of input. These are all the functions in which the listing shows more conditional branches, compares or panic call sites than before, and why: | Function | Listing | Why | |---|---|---| | `compute_color_depth` (`bun_core`) | 385 to 408, +6 branches, +6 compares | `index_of` got shorter (change 12), and the compiler now inlines it here. Runs once per process. | | `EncodedSlice::fmt`, `FormatUTF16::fmt` (`bun_core`) | 382 to 365 and 389 to 380, +1 panic site each | They called `SharedTempBufferBorrow::new` and its `drop`, which each looked the thread local up. What those two did is now inline (change 8), with the panic for a destroyed thread local that both contained. 5 branches fewer each. | | `WrappedWriter::write_16_bit` (`bun_runtime`) | 100 to 162, +9 branches, +9 compares, +6 calls, +3 panic sites | The same: the two functions that it called, of 35 and 38 instructions, are inline. It asks for the address of the thread local once instead of twice. | | `StructuralIndex::seek_refill`, for `u8` and `u16` (`bun_parsers`) | 35 to 36, +1 branch, +1 compare | After `fill_to` has returned, `at` compares the index with the length of the window again, where it read the element unchecked (changes 5 and 51). | | `load_node_modules` (`bun_resolver`) | 2,828 to 2,848, +3 branches, +3 compares | The compiler duplicates three tests at exits: twice the one in front of a `dealloc`, once the one of the debug log's indent. One per path, as before. | | `finalize_result`, `load_as_file_or_directory` (`bun_resolver`) | 735 to 712 (+1 branch, −3 compares, +1 call) and 564 to 559 (−1 branch, +1 compare) | `Option<&PackageJSON>` is one word where `Option<*const _>` was a tag and a pointer (change 26). In `finalize_result` the compiler duplicates one call of `BSSStringList::append`. | | `load_tsconfig` (`bun_resolver`) | 481 to 502, +1 compare, +5 calls, −1 panic site | The drops of the boxed configs (change 27): three call sites of their drop code and two of `dealloc`. An `unwrap` goes. | | `h3::ClientContext::get_or_create` (`bun_http`) | 74 to 84, +2 branches, +2 compares, +1 panic site | It reads the loop itself (change 21). The same two tests leave its caller `HTTPClient::start_::<true>` (940 to 928), and they now run only when the engine is created. | | `add_catalog::prepare`, one instance of `reject_unknown_update_requests`, `update_package_json_and_install_with_manager_with_updates` (`bun_install`) | 939 to 937 (+2, +2), 512 to 516 (+2, +2), 3,702 to 3,722 (+2 branches, −2 compares, +5 calls, +1 panic site) | Their source is not touched. They walk, copy and drop `UpdateRequest`s, whose size and field order change. The third has the same 25 allocations, and four more copies of the `dealloc` in its drop code. | | `package_json_editor::edit` (`bun_install`) | 3,005 to 3,003, +2 branches, −2 compares | `Option<StoreRef>` is one word where `Option<*mut _>` was two (change 42). | | Change | What differs | Effect | |---|---|---| | 1 | `toml::Parser::parse_keyval` goes from 275 to 282 and `TOML::parse` from 601 to 602: moves. | Same tree. | | 2 | `ZStr::boxed` and the two methods of `OptionsEnvArg for Box<ZStr>` go, 109 instructions. | Nothing called them. | | 3 | The two `structural_index_chunk` are still inline in their callers, which do not change. A copy of each (10 instructions) is now also emitted on its own, and nothing in the crate calls it. That is +20 of the +30 instructions of `bun_parsers`, and its 2 calls and 2 panic sites. | None. | | 4, 13, 14 | The crash handler goes from 825 to 819: two `strlen` calls and their branches go. `CrashHandler__unsupportedUVFunction` goes from 48 to 46, without the test for null. Because it is known not to return, `napi::uv_posix::unsupported` (6) is inlined in the six `uv_*` stubs that call it, without its `unreachable!`. One closure of the crash handler and one of `__runtime_embed_load` have another number in their names. | Only on the way to a crash. | | 7 | `extend_from_fn::<Dependency>` (69) and `::<u32>` (40) are `Vec::extend_trusted` (57 and 33). | One bounds check fewer per call. | | 8 | Apart from the three functions above: `format_utf16_type::<Formatter>` (150) is `with_shared_temp_buffer` with its closure (134). `SharedTempBufferBorrow::new` (35), its `drop` (38) and the slot's `drop` (9) go. The thread local's destructor goes from 10 to 8, and `bun_runtime` gets a copy of it (6). | Same text. After a nested use the inner buffer is freed, not the outer. | | 9 | **`SmolStr::from_char` (3 instructions) was inline in the shell's brace lexer, which is in other crates, and is a function that they call.** rustc offers a body to other crates by itself only while it makes no call, and this one now calls `unwrap` until LLVM folds that away. `braces::Token::to_text` goes from 37 to 39 and ends three arms in a jump to it. The two `tokenize` make one call more each (581 to 581 and 669 to 676). | One call per `{`, `}` or `,` token in this comparison, which does not link. The build that ships inlines across crates. | | 12 | In `index_of` (44 to 39) the second test of `haystack.len() < needle.len()` folds away. `SplitIterator::next` goes from 93 to 91 and `StringJoiner::push_node` from 82 to 81. `replace` keeps its 122. | Same index. | | 16 | The four `Source::configure_*thread*` functions are 5 shorter each, with 2 compares fewer. The compiler had reduced each static to a flag and rebuilt the descriptor from it with a compare. Now it loads the descriptor. `stdio::init` keeps its 211. | Same descriptors. | | 17 to 20, 25 | `check_package_path` goes from 775 to 725 and `check_relative_path` from 766 to 747 (both with change 26). `add_entry_with_store` goes from 572 to 570, without the store of `dir`. `read_file_contents` keeps its 709. | Same results. | | 22 | `ssl_ctx_setup` (18) and `HTTPContext::<true>::init_with_opts` (49) keep their length. | Same calls. | | 23, 24 | `InternalSourceMap::find` goes from 176 to 177, `append_vlq_to` from 154 to 161 and `Builder::finalize` from 209 to 217: moves to and from stack slots. `seed_window` goes from 77 to 72. No compare or branch more. | The two loops compute the stream once, not once per window. | | 26 | Apart from the sizes: `handle_esm_resolution` goes from 487 to 481, `resolve_and_auto_install` from 1,305 to 1,300, `get_loader_and_virtual_source` from 368 to 365, and the `read_dir_info_package_json` thunk from 26 to 22. | Same package.json. | | 28 | `bin_dirs` goes from 18 to 11 and `dir_info_uncached` from 1,099 to 1,089, without the loads and stores of the flag. | Same list. | | 29, 30 | `LazySourceMap::load` goes from 230 to 219. `Lookup::get_source_code` goes from 180 to 189, with the same tests in another order: it looks at the new field before it asks for a provider. `SourceContentPtr::from_provider` (6) goes. `to_executable` is 2 shorter, `target_executable` 1 longer. | Same sources. | | 31 | `http_thread_on_init_error` goes from 107 to 99. **The empty default of the field is the address of a static NUL byte, where it was a constant. Each of the 11 functions that build a default `InitOpts` needs one `lea` more for it:** `preconnect` and `send_sync_for_result` in `bun_http`, and 9 in `bun_runtime`, `FetchTasklet::queue` among them. | One instruction more per call, so per `fetch()`. | | 33, 46, 49, 50 | `init` goes from 2,339 to 2,275 (with 31), with 8 branches, 5 calls and 2 panic sites fewer. `allocate_package_manager` is 4 longer and the drop of `PackageManager` 1. `create_fake_temporary_node_executable` goes from 541 to 518. The readers of the path load it from the manager: between 4 shorter and 3 longer. | One copy of the cwd fewer. | | 36 | `find_workspace_root_above` goes from 1,286 to 1,276. | One copy and one bounds check fewer per package.json above the cwd. | | 39 | `StringBuilder` loses a pointer that release builds computed and stored and never read. `allocate` goes from 62 to 60. Three functions that have the builder inline, and that also copy `Dependency`s (41), have fewer branches and panic sites: `Package::clone` (1,130 to 1,105, −6 and −2), `fetch_necessary_package_metadata_after_yarn_or_pnpm_migration` (1,055 to 1,042, −2 and −1) and `get_or_put_resolved_package_with_find_result` (3,118 to 3,123, −6 and −2). | Same strings. | | 40 | The guard's `drop` (21) has another name. `install_with_manager` goes from 3,871 to 3,864 (with 7, 35 and 47). | Runs at the same point. | | 41 | `parse_with` goes from 858 to 848, `Group::clone` from 38 to 34, `default` and `from` lose 2 each. `parse_dependency` goes from 1,842 to 1,844, with one panic site fewer. | Same ranges. | | 42 | `edit_after_resolve_slow` goes from 3,471 to 3,447 and `move_to_front` from 144 to 123. | Same package.json. | | 43 | `tarball_stream::drain_callback`, which has `populate_result` inline, goes from 1,872 to 1,859, with 5 branches fewer. | Same files. | | 47, 48 | `Diff::generate_inner` goes from 5,192 to 5,193, with 4 branches and 9 compares fewer. `parse_with_json` goes from 4,287 to 4,294, with 5 branches fewer. | Same diff. | Changes 5, 21 and 27 are in the table of checks. Changes 3, 6, 10, 11, 34, 35, 37 and 38 ask for the same operations in the same order. Where they show up at all, it is in registers: `save_lockfile` goes from 294 to 292, and `run_tasks_erased` keeps its 6,254. Changes 15, 32, 44 and 45 are not compiled on Linux. The comparison covers x86-64 Linux only, one crate at a time. The build that ships also inlines across crates when it links. ### The changes in detail #### 1. toml: the parser keeps tables and arrays as `StoreRef` `e_object()` and `e_array()` return a `StoreRef`, which is `Copy` and derefs to the node. The parser called `.as_ptr()` on it, passed `*mut E::Object` / `*mut E::Array` around and dereferenced the pointer in six `unsafe` blocks. It keeps the `StoreRef` instead, as the YAML and XML parsers do. The `meta` map is still keyed by the node's address. #### 2. bun_core: remove `boxed_zeroed_unchecked`, `ZStr::boxed` and `OptionsEnvArg for Box<ZStr>` Nothing calls `boxed_zeroed_unchecked`. `ZStr::boxed` is called only by `impl OptionsEnvArg for Box<ZStr>`, and `append_options_env` is used only with `&'static ZStr`, `String` and `OptionWord`: no `Box<ZStr>` exists anywhere else. One `unsafe fn` and three `unsafe` blocks go with them. #### 3. xml: `Unit::from_u16` and `Unit::from_bytes` instead of casts The scanner is generic over `u8` and `u16` code units. Three places that run for one width only turned a value of the concrete type into the generic one, or back, with `transmute_copy` or `from_raw_parts`. `find_ascii` uses `Unit::bytes`, which is the identity for `u8`. The other two are trait methods: the identity in the impl for the width they are reached with, `unreachable!()` in the other one, where `units_of` had `assert!(!U::WIDE)`. Each caller is behind the constant `U::WIDE`. #### 4. crash_handler: `UNSUPPORTED_UV_FUNCTION` holds the name's bytes `CrashHandler__unsupportedUVFunction` stored the `*const c_char` it was given in a thread local, and the two places that print it turned the pointer into bytes again in an `unsafe` block each. The function already measures the string for its own panic message. It does that first and stores the `&'static [u8]`: it never returns, which its signature says with `-> !`, so the string outlives every read. The readers take the slice as it is. The arm for a null `name` goes: the function's contract excludes null, and it passed `name` to `cstr` a few lines further down anyway. #### 5. xml_index: `StructuralIndex::seek` and `at` read the window with `get` Both compared an index with `win.len()` and then read the element with `get_unchecked`. `slice::get` is that comparison and that read. In `seek` the loop condition becomes `while let Some(&p) = win.get(rel)`. `at` calls `fill_to` when `get` misses and tries again: `fill_to` returns only once the index is in the window. `seek_refill` called `at` for every index. It calls `fill_to` and then walks the window as `seek` does, so it reads `base` and the window once per fill. #### 6. collections: `HiveSlot` borrows its hive `HiveSlot<'h, ..>` kept the hive as a `*const HiveArray` that is null for a heap slot, next to a `PhantomData<&'h HiveArray>`, and its `Drop` dereferenced the pointer in an `unsafe` block. That pair is `Option<&'h HiveArray>`: the same word with the same null test, and `'h` was borrow-checked already. All of `HiveArray` is in `UnsafeCell` or `Cell`, and `HiveBox` holds the same reference. `HiveArray::drop_all` was an `unsafe fn` with `Drop` as its only caller. Its body moves there, with the `SAFETY` comment of the call on the `drop_in_place` that needs it. The comment also names what it relies on where a token is forgotten (`assume_init`, `ManuallyDrop`): that the slot has been written. #### 7. bun_core: `vec::extend_from_fn` is `Vec::extend` It reserved, wrote `f(i)` into `spare_capacity_mut()[..n]` and called `set_len` in an `unsafe` block. `(0..n).map(f)` is a `TrustedLen` iterator, for which `Vec::extend` does the same: one `reserve`, raw writes, and the length kept in a local until the end. The check of `spare[..n]`, which the compiler could not remove, goes away. If `f` panics, the elements written so far are in the `Vec`, which is what the doc comment says. #### 8. fmt: the shared temp buffer is an `Option<Box>` The thread local held `Option<NonNull<[u8; 32 KiB]>>` in a wrapper type with hand-written `Drop` and `Deref`, and a guard type took the pointer out, dereferenced it, and on drop put it back or freed it: three `unsafe` blocks for what `Option<Box<_>>` does by itself, in the same word. `with_shared_temp_buffer` takes the `Box` out of the cell (or allocates one), lends it to a closure and puts it back. The three users run their body in that closure. While one runs the cell is empty, so a writer that formats UTF-16 or Latin-1 again still gets a buffer of its own. After such a nested use the inner buffer is the one that is freed, not the outer. #### 9. SmolStr: `Inlined::all_chars` views the `u128` with `bytes_of_mut` It cast `&mut self.0` to `&mut [u8; 15]` through a raw pointer. `bytes_of_mut` gives the 16 bytes of the `u128`, as `slice()` above does with `bytes_of`, and `first_chunk_mut` the first 15. The length is a constant, so the `unwrap` folds away. `Inlined::ptr` had no other caller. #### 10. bun_core: `impl io::Write for Writer` calls the inherent methods `write_all` and `flush` of the trait impl repeated the bodies of the inherent `Writer::write_all` and `Writer::flush`, `unsafe` block included. They call those instead. All four are `#[inline]`. #### 11. atomic_cell: `unreachable!()` for the widths that `Atom` excludes `size_dispatch!` matches on `size_of::<T>()`, a constant in every instantiation, so an optimized build drops the arm for other widths however it is written, and `unreachable_unchecked()` bought nothing. #### 12. highway: `memmem` subtracts addresses The index of the match is the pointer that `highway_memmem` returns minus the start of the haystack. For `u8`, `offset_from` is that subtraction and nothing else, and subtracting the two `addr()`s needs no `unsafe`. It is the same `sub` where overflow checks are off. #### 13. Check constant strings at compile time instead of `from_utf8_unchecked` The crash handler's `GIT_SHA` is the first 7 characters of a `&str` constant: `str::split_at` is a `const fn`, so the detour through bytes and `from_utf8_unchecked` is not needed. `env::GIT_SHA_SHORT`, the first 9 characters, made the same detour through `const_str_slice`, with `core::str::from_utf8`. It uses `split_at` too, and `const_str_slice`, which had no other caller, goes. `__runtime_embed_load` turned the `&[u8]` constants `CODEGEN_PATH` and `BASE_PATH` into `&str` at run time without a check. They become `&str` constants through the `const fn` `core::str::from_utf8`, so that a path that is not UTF-8 fails the build. The doc comment of `strings::str_utf8` names that use instead of `const_str_slice`. #### 14. Call `gettid`, `getpid`, `alarm`, `_exit` and `raise` as `safe fn` Seven `unsafe` blocks in `bun_core` and the crash handler only called a libc function that takes integers or nothing and has no memory-safety precondition. The other OS arms of `thread_id::current_uncached`, and `Global.rs`, `spawn/process.rs` and `ParentDeathWatchdog.rs`, declare such functions as `safe fn` in an `unsafe extern "C"` block next to the caller. These sites do the same, with the signatures that the `libc` crate has, so they call the same symbols (`gettid` still goes through `--wrap=gettid`). For `getpid`, `debug.rs` uses the declaration that `output.rs` has. #### 15. bun_core: `fd_path_raw_w` takes `&mut [u16]` It was an `unsafe fn` that took a pointer and a capacity, and both callers passed `as_mut_ptr()` and `len()` of a slice. With the slice, only the call to `GetFinalPathNameByHandleW` is `unsafe`: the `\\?\` prefix is read by index and removed with `copy_within`. Every index comes after a comparison of `n`, which is the length of the slice, with a larger constant, and each `copy_within` range after one with a constant no smaller than its start, so no bounds check is left. That is why `copy_within` is called in each of the two arms and not once with the offset they produce. #### 16. output: `STDOUT_STREAM` and `STDERR_STREAM` are atomics Each of the two statics holds a file descriptor that `set_init` writes once at startup and that every thread reads when it is configured. They were `RacyCell<File>`, read and written in `unsafe` blocks. An `AtomicI32` (`AtomicU64` on Windows, where an `Fd` is 64 bits) with `Relaxed` loads and stores is the same plain load and store, as for `bun_stdio_tty` below. #### 17. resolver: `BrowserMapPath::map` is `&'static BrowserMap` The one constructor passes `&package_json.browser_map`, and `DirInfo::package_json()` already returns `&'static PackageJSON`. The field shortened that to `'b`, so each of the four matches in `check_path` widened the value back with `Interned::assume`. With the real lifetime on the field, `map.get()` yields a `&'static Box<[u8]>` that is assigned to `remapped` as is. #### 18. resolver: keep the slot that `MaybeUninit::write` returns `dir_info_cached_miss` wrote a queue item with `queue[i].write(..)`, dropped the `&mut` that call returns, and a few lines later got the same slot back with `unsafe { queue[i].assume_init_mut() }`. Only `rfs.entries.get(top)` runs in between and it does not touch `queue`, so the returned reference is kept. #### 19. resolver: drop three lifetime casts that change nothing `BSSStringList::append` and `print` return a slice whose lifetime the caller picks, so `FilenameStoreAppender::append` and the stores' `print` already had the lifetime they return; the `Interned::assume(..).as_bytes()` after each call was a no-op, as `append_slice` right above shows. `add_entry_with_store` dereferenced `stored` twice, once behind a `&*ptr::from_ref(..)` of a slice that already had an unbounded lifetime. It now makes one `&'static Entry` and uses it for the key and for the return value. #### 20. resolver: `RealFS::read_directory` and `EntryLookup` keep their `'static` `RealFS::read_directory` forwards to `read_directory_with_iterator`, which returns `&'static mut EntriesOption`, but declared the result as a borrow of `&mut self`. `FileSystem::read_directory` and `DirEntryAccessor::openat` cast it back to `'static` through a raw pointer. It now returns what it gets, and both callers use the value as it comes. `EntryLookup<'a>` tied the `&Entry` it hands out to the `DirEntry` it was looked up in, while its own safety note rests on the entry being a slot of the never-freed `EntryStore`, the only source of the pointers in `DirEntry::data`. `EntriesOption::lookup` widened a `&DirEntry` to `'static` only to get an `EntryLookup<'static>` out of `get`. The parameter is gone and `entry()` returns `&'static Entry`. One `&mut Entry` could overlap such a reference: `add_entry_with_store` borrowed a recycled entry mutably to store `dir`. All four callers that pass a previous map build the new `DirEntry` with a copy of the old one's `dir`, and an entry's `dir` is only ever a copy of its `DirEntry`'s, so the store wrote the pointer and length that were already there. The store is gone and the entry is borrowed shared; the rest of that block uses `mutex`, `need_stat` and `cache()`, which take `&self`. #### 21. http: `h3::ClientContext::get_or_create` reads the loop itself Both callers passed `unsafe { NonNull::new_unchecked(http_thread().uws_loop) }` and the callee only unwrapped it again with `as_ptr()` for the FFI call. The parameter is gone; the existing FFI block passes `http_thread().uws_loop()`, and only when the engine has to be created. #### 22. boringssl: `ssl_ctx_setup` takes `&OwnedSslCtx` It was an `unsafe fn` over `*mut SSL_CTX` whose only requirement was that the context is live. Its two callers, both in `HTTPContext`, had just stored an `OwnedSslCtx` in `self.secure` and fetched the raw pointer back through an `unwrap`. `OwnedSslCtx` owns a reference, so taking it by reference puts that requirement in the type and the function is safe. The callers pass what `self.secure.insert(..)` returns, and `HTTPContext::ssl_ctx` is gone. #### 23. sourcemap: `impl_pod!` for `SyncEntry` and `ObjectBrace` `Builder::finalize` viewed both tables as bytes with `slice::from_raw_parts`, and `for_each_object_brace` read each `ObjectBrace` with `ptr::read_unaligned`, each behind a comment that the struct is `repr(C)` with no padding. `impl_pod!` has rustc prove that neither struct has padding and that every bit pattern is a value of it, so the views are `slice_as_bytes` and the read is `read_prefix`. The macro does not look at the `repr`: both structs and `LineColumnOffset` are `repr(C)`, and `Ordinal` is `repr(transparent)`. The byte lengths come from those slices, so `copy_from_slice` compares the same values as before, and `read_prefix` is given a `&[u8; 16]`, so its length test is a constant. #### 24. sourcemap: `WindowReader<'a>` borrows the stream `WindowReader` kept the stream as `*const [u8]` and handed it out through `fn bytes<'a>(&self) -> &'a [u8]`, an unbounded lifetime from a raw pointer. Every reader is a local of `find`, `for_each` or `append_vlq_to`, so the field is `&'a [u8]`: copying the reference out leaves `self` free to mutate, which is what the accessor was for. A reader started out as `DANGLING`, with a null stream, and `parse` stored the stream it was given. A reference cannot be null, so `WindowReader::new(stream)` takes the stream, and `parse` reads it from `self`. The two loops compute the stream once instead of once per window, and so also for a map with no windows. That is three header loads that are valid for every blob; `object_braces()` already does them unconditionally. #### 25. resolver: four `unsafe` blocks that plain borrows replace - `Resolver::standalone_module_graph` is `Option<&'static dyn ..>`. Its only two setters, in `VirtualMachine`, assign a `'static` graph, so the transmute in `for_worker` that widened `'_` to `'a` is a copy. `for_worker` stays `unsafe` for `env_loader`. - The close-or-store guard in `RealFS::kind` captures `&mut cache.fd` instead of a `*mut EntryCache`; nothing else touches `cache` while the guard lives. - `FileSystem::init_with_force` returns `INSTANCE.get().cast()` when already loaded, instead of going through a `&mut MaybeUninit` for the same address. - `read_file_with_handle_impl` writes the trailing NUL through `spare_capacity_mut().first_mut()`: the same compare and the same store. #### 26. resolver: `package_json` travels as `&'static PackageJSON` `Result::package_json`, `MatchResult::package_json` and the `read_dir_info_package_json` hook held `Option<*const PackageJSON>`. Every value stored there comes from `DirInfo::package_json()`, `DirInfo::enclosing_package_json` or `FallbackModule::package_json`, which are `&'static PackageJSON` already, so each store cast a reference to a pointer and each read cast it back with `unsafe { &*p }`. The reason given for the pointer was that zeroed initialisation stays valid. Nothing zero-initialises these structs, their `Path` fields hold slices that could not be zero anyway, and an all-zero `Option<&T>` is `None`. The fields and the hook hold `Option<&'static PackageJSON>`, the four functions that store their `package_json` argument take `&'static`, and `Result::deref_package_json` is gone. Both structs shrink by 8 bytes. #### 27. resolver: `load_tsconfig` keeps its boxes Each `Box<TSConfigJSON>` from `parse_tsconfig` was turned into a raw pointer, kept in a `BoundedArray<*mut _, 64>`, read through a `BackRef`, dereferenced twice per merge step and turned back into a box twice. Nothing but this walk holds the configs, so they stay boxes: `current` owns the newest one and the array owns the ones before it, 63 of them, so a chain is still cut off at the 65th config. Two `expect`s and an `unwrap` go with the pointers. When a chain is that long, the configs read so far are freed with the array instead of leaked. #### 28. resolver: `BIN_FOLDERS` is initialised at compile time The static was a `RacyCell<MaybeUninit<BinFolderArray>>` with a `BIN_FOLDERS_LOADED` flag beside it, because `BoundedArray` had no const constructor: both append sites first wrote an empty array under the flag, and every access went through `assume_init_*`. `BoundedArray::new()` is a `const fn` with the body of `default()`, so the static starts as an empty array. The flag, its three loads and both initialising blocks are gone; `bin_dirs()` returns the empty slice of that array where it returned `&[]`. #### 29. standalone_graph: in `inject`, `zname` borrows `zname_owned` The two places that repoint `zname` at a heap buffer stored the box in `zname_owned` and then rebuilt a `&ZStr` from its raw pointer and length, with a comment that the box outlives it. `zname` now borrows what `zname_owned.insert(..)` returns, so the compiler checks that: the old `zname` is last used to build the new buffer, before `zname_owned` is overwritten. The `let _ = (&mut zname_owned, &mut zname);` that keeps Windows from warning about two variables it never changes is compiled for Windows only, since it would borrow `zname_owned` again while `zname` is in use. #### 30. sourcemap: `ParsedSourceMap` holds the standalone graph's sources by type The standalone graph boxed its own `SerializedSourceMapLoaded`, cast the pointer to `*const SourceProviderMap` and stored it as an integer in `underlying_provider`, with `is_standalone_module_graph` set so that nothing would call it as a provider. `Lookup::get_source_code` cast that integer to `SerializedSourceMap::Loaded`, a different `repr(Rust)` struct with the same fields, and dereferenced it. The flag is replaced by `standalone_module_graph: Option<&'static Loaded>`. The graph builds a `Loaded` and leaks it, as it leaked the box before, and `get_source_code` reads through the reference, before it asks for a provider, which such a map no longer has. The duplicate struct, `standalone_module_graph_data` and `SourceContentPtr::data` are gone, and so is `SourceContentPtr::from_provider`, which only forwarded to `from_source_provider` and whose one remaining caller calls that instead. #### 31. http: `InitOpts::abs_ca_file_name` is a `&'static ZStr` The field was a `&'static [u8]` that was NUL-terminated by contract. `on_init_error_noop` rebuilt a `ZStr` from its pointer and length in an `unsafe` block, while the one producer, `PackageManager::init`, stored a slice whose length included the NUL, and its own error handler stripped it again. The producer already holds a `ZBox`, so it parks that in `ABS_CA_FILE_NAME` and passes the `&ZStr`. Both handlers use the field as it is. #### 32. exe_format: `_mh_execute_header` is a `safe static` `find_identity` on macOS copied the header out of the extern static with `unsafe { header_address.read() }`. The linker defines the symbol for every executable, the header is eight integers, and it stays mapped read-only for the life of the process, so reading it has no precondition. The declaration says so with `safe static` and the header is read by value. The block that views the load commands after it stays. #### 33. install: `init` reborrows the `&'static mut DirEntry` it matched on `FileSystem::read_directory` returns `&'static mut EntriesOption`, and its `Entries` arm holds a `&'static mut DirEntry`. `init` and `init_with_runtime_once` turned the matched reference into a raw pointer and back in an `unsafe` block to get a `'static` lifetime that a plain reborrow already has. Both now write `&mut **e`, as `init_with_runtime_once` does further down for `manager.root_dir`. #### 34. install: `save_lockfile` uses `progress.root` instead of a stashed node pointer `Progress::start` returns `&mut self.root`. `save_lockfile` kept that reference as a `*mut Node`, initialised to null, and dereferenced it in two `unsafe` blocks. The first use is now chained onto `start()`, and the second is `this.progress.root.end()`, which is the same node and the spelling the function already uses two lines further down. #### 35. install: drop four raw-pointer and lifetime detours that borrowck does not need - `install_with_manager`: `maybe_root.parse` got `&mut *mgr` through a raw pointer. Its other arguments are a stack `Lockfile`, the detached `log_mut()` and an owned source, so `manager` can be passed directly, as `bun link` does. - `run_tasks`, extract arm: `alias` was `detach_lifetime(tarball.name.slice())` next to `resolution`, a plain borrow of the same `tarball`. While it is live the arm only touches `task.data`, `Copy` fields and `&self` accessors, and makes shared reads of `task.request`. - `run_tasks`, manifest arm: `name` was detached from `task.callback`. Every other use of `task` in the arm is a different field, except `retry_with_abbreviated_manifest(&mut self)`, whose two arguments are now computed first, in the same order. - `populate_manifest_cache`: `(*mgr).end_progress_bar()` after `sleep_until` returned is `manager.end_progress_bar()`; `mgr` is not used afterwards. #### 36. install: `find_workspace_root_above` borrows `walk.path` for the source path Each package.json above the cwd had its path copied into the process-global `ROOT_PACKAGE_JSON_PATH_BUF` in an `unsafe` block, only to name the `Source` that is parsed in the same loop iteration. `Source<'a>` already borrows the loop-local `json_buf`, the body does not touch `walk` again, and log locations own their bytes, so the source now borrows `walk.path`. This removes one copy and one bounds check per package.json. That check could not fail: the copy was only reached after `openat_a` had opened the path, and `openat_a` refuses a path that does not fit a `PathBuffer`. The global buffer is left with a single writer, `init`. #### 37. install: `AutoInstaller::path_for_resolution` takes `&mut PathBuffer` The trait method took `&mut [u8]`. Its only implementation debug-asserted the length and cast the slice's pointer back to `&mut PathBuffer` in an `unsafe` block, and its only caller, the resolver, passes `bufs!(path_in_global_disk_cache)`, which is a `&mut PathBuffer` that was deref-coerced to a slice. The parameter now has the type both ends use, so the cast, the assert and a length that release builds never checked are gone. The call in the resolver is unchanged. This was the last place that reinterpreted a `&mut [u8]` as a `PathBuffer`, so the sentence in the `PathBuffer` doc that gave such casts as the reason for its alignment is gone too. #### 38. install: `NetworkTask` reads its manager through the `ParentRef` it stores `pm_mut()` turned the `package_manager` back-reference into a `&'a mut PackageManager` with an unbounded lifetime. Its two users, `manifest_request` and `for_tarball`, only call `&self` methods and read fields, while their callers hold the manager mutably. They now copy the `ParentRef`, which is `Copy` and derefs to `&PackageManager`, into a local, so `self` stays free to be assigned to. The accessor is deleted. #### 39. install: remove three one-line `unsafe` blocks that have a safe spelling - `lockfile::StringBuilder.ptr` was an `Option<*mut u8>` computed with `as_mut_ptr().add(prev_len)` and only ever asked `is_some()`, in debug assertions. It is now `allocated: bool`. - `Task::Id::for_npm_package` hashed the bytes of a `semver::Version` through `ffi::slice`. `VersionType<u64>` is `NoUninit`, so `bun_core::bytes_of` yields the same bytes. - `__bun_regex_compile` takes a reference and returns an `Option`, and its definition in `bun_jsc` is a safe fn, so the `extern "Rust"` declaration in `bun_install_types` says `safe fn`. #### 40. install: clear `root_lifecycle_scripts` with a `scopeguard` on `manager` `install_with_manager` declared a local `ClearRootLifecycleScripts(*mut PackageManager)` whose `Drop` wrote through the raw pointer. The pointer was taken from `manager`, and `manager` itself was used for the rest of the function, so the pointer was stale by the time the guard ran. The guard now owns the `&mut PackageManager` and the rest of the function reborrows it from the guard. It is declared at the same place, so it runs at the same point. #### 41. semver: delete `query::Group.input` `Group` stored the parsed text as a lifetime-erased `*const [u8]`. The only read was in `Package::parse_dependency`, an `unsafe` deref in the same function that had passed that slice to `parse`; `bun lint` overwrote the field after parsing so that it would not dangle. `parse_dependency` now keeps the slice next to the range, in `workspace_range: Option<(Group, &[u8])>`, and the field is gone, which makes `Group`, and every `DependencyVersion` and `Dependency` with it, 16 bytes smaller. #### 42. install: `UpdateRequest.e_string` keeps the `StoreRef` it is made from The field was `Option<*mut E::String>`. Every writer got a `StoreRef<E::EString>` from `Data::e_string()` or `string_entry()` and called `.as_ptr()` on it, and every reader dereferenced the raw pointer in an `unsafe` block. `StoreRef` is the AST's own `Copy` handle, with `Deref`, `DerefMut` and pointer-identity `PartialEq`, and `edit` already writes through one a few lines below. The field now stores the handle, so the writers drop `.as_ptr()`, the readers in `edit` and `add_catalog` use it directly, and the two comparisons compare handles. #### 43. install: `TarballStream::init` returns a `Box` and `populate_result` is a safe fn - `init` ended in `heap::into_raw(Box::new(..))`, and both callers passed the pointer straight to `heap::take`. It now returns the `Box`. - `populate_result` was an `unsafe fn` taking `*mut Task`, with its whole body in one `unsafe` block. All it needs from the task is the tarball of the `extract` request and the log, and all it leaves there is `data`, `err` and `status`. It now takes `&ExtractTarball` and `&mut Log` and returns `Result<ExtractData, Error>`, with no `unsafe` inside. `finish`, which is `unsafe` anyway and reads the same union field again further down, reads the union and stores the three fields. - The entry path was built with `OSPathSliceZ::from_raw_mut`, although only the Windows escape pass writes to it, and that pass starts by asking for the slice. The checks and the escape pass now work on `&mut norm_buf[..norm_len]`, and the NUL-terminated view is made afterwards with `from_buf`. That leaves `WStr::from_raw_mut`, `WStr::as_mut_slice` and `DerefMut for WStr` without a user, so they are deleted. - The comments in `init` and `finish` name the function that sets the `extract` variant, and describe how the stream is really freed: by dropping `network_task.tarball_stream.take()`. #### 44. install: the Windows hardlink task is passed and reclaimed as a `Box` `HardLinkWindowsInstallTask::init` returned `into_raw(Box::new(..))` for an `unsafe fn push(*mut T)`, and `run_from_thread_pool` kept the raw pointer: it called `run()` and stored `err` through it, and reclaimed the `Box` on two separate paths. The worker owns the task from the first line of the callback, so it now takes the `Box` back right after the `complete_one` guard, as `UninstallTask::run` does. `init` returns the `Box`, and `push` takes it, is a safe fn, and requires `TaskType: Send + 'static`, like `OwnedTask`. The drop order is unchanged: the slot lock, the task, then `complete_one`. #### 45. install: the Windows `Hardlinker` keeps the cwd slice it gets `get_fd_path_w` returns a sub-slice of `cwd_buf`. `link` turned it into an offset, with `offset_from`, and a length, and sliced the buffer again for every file, because the slice supposedly could not stay borrowed across the loop. Nothing else uses `cwd_buf`, so the slice is now kept and used directly. This also saves one bounds check per linked file. #### 46. install: `ROOT_PACKAGE_JSON_PATH` becomes `PackageManager::root_package_json_path` The path was a `pub static RacyCell<&ZStr>` that `init` wrote and eight places read in `unsafe` blocks. Every reader has the manager at hand, so the path is now a field that `allocate_package_manager` fills in. `init` passes the `ZStr` it builds in the static buffer, and auto-install passes `ZStr::EMPTY`, the static's initial value, which `root_patched_dependencies` relies on to fall back to the package.json next to the lockfile. A process allocates one manager: a second `init` only follows `MissingPackageJSON`, which is returned before the path is built and before the manager is allocated, and `init_with_runtime` runs under a `Once`. #### 47. install: `Diff::generate` takes `to_update: bool` instead of the update requests `install_with_manager` passed `Some(&manager.update_requests[..])`, made in an `unsafe` block, next to `&mut *manager` whenever `manager.to_update` was set, and `None` otherwise. `generate_inner` only asked the slice whether it was empty, which it also asks `pm.update_requests` itself at its top. The parameter is now the bool, and the emptiness is read once from `pm` and used in both places. The callers that passed `None` pass `false`. The SAFETY text at that call also says that `Diff::generate` never reaches `manager.lockfile` through `pm`. That was false with `--frozen-lockfile` and for `bun pm`: `parse_with_json` built `SkipIfInLockfile(&pm.lockfile)` on every parse, while `from_lockfile` was a live `&mut` to the same `Lockfile`. Only the two `process_names_array` calls of the `workspaces` group consume the value, so it is now built only under `FEATURES.workspaces`, which the parses that `Diff::generate` and the folder resolver make do not set. The behaviour is the same. The comment in folder_resolver.rs states that property, and the one in prune.rs no longer claims it for its own `Features::main()` parse, which does read `pm.lockfile`, at a point where nothing else borrows it. #### 48. install: `Package::parse_from_real_manager` takes `&mut PackageManager` It was an `unsafe fn` taking `*mut PackageManager`, and its one caller passed `ptr::from_mut(self)` in an `unsafe` block. The contract asked for a live manager whose `lockfile` and `log` point to live allocations outside it. A `&mut PackageManager` gives the first, `lockfile` is a `Box`, and `log` comes from the safe `log_mut()`. What is left is that the `&mut Lockfile` it splits off must be the only way to the lockfile during the parse. `parse_with_json` derefs `pm.lockfile` only under `Features::workspaces`, so the `features` parameter is gone and the function always parses with `Features::NPM`, which is what its caller passed. With that nothing is left for a caller to promise, and the function is safe. One `unsafe` expression remains inside it, the `&mut *lockfile`. #### 49. install: in `init`, `original_cwd` borrows the boxed copy of the cwd `original_cwd` was a slice of `original_package_json_path_buf`, reached through `ZStr::from_raw` so that the borrow checker would not see it. The directory walk, `this_cwd` and a later pointer comparison all hang on that slice, and they are still in use when the buffer is truncated and gets `/package.json` appended again. If package.json is in a parent directory, that write lands inside `original_cwd`. `init` already makes a boxed copy of the cwd to return it. The copy is now made from `top_level_dir_no_trailing_slash`, the bytes the buffer was filled with, and `original_cwd` borrows it, so the walk no longer points into the buffer. The first `original_package_json_path` is only read before the buffer changes and is reassigned before its next use, so it can be a plain `from_buf` borrow. Nothing read the overwritten bytes, so the behaviour is the same. #### 50. install: remove two pieces of dead code that needed `unsafe` - `create_fake_temporary_node_executable` turned a non-empty `optional_bun_path` into a `ZStr` with `from_raw`, trusting the caller of a safe `pub fn` to have put a NUL behind the slice. Both callers pass an empty slice and only read what the function writes back, so the branch is gone. - `init` copied the cwd into `CWD_BUF` right after reading it. Nothing reads the buffer before `init` overwrites it further down and publishes it as `top_level_dir`, so the first copy is gone. #### 51. xml_index: `seek_refill` calls `at` for every index, as it did The commit that made `seek` and `at` read the window with `get` also gave `seek_refill` a loop of its own over the window. That left `at` without a caller outside the unit tests and the benches, which the `unused_pub` lint reports. `seek_refill` has its old body again. Compared with `main`, `seek_refill` is one instruction, one compare and one branch longer (35 to 36 instructions, for `u8` and for `u16`): `at` compares the index with the length of the window again after `fill_to` has returned. That happens once per refill. #### 52. resolver: `EntriesOption::try_entries_mut` returns the listing or the read error `init` and `init_with_runtime_once` of the package manager turned the result of `read_directory` into a `&'static mut DirEntry`, or returned its error, with one `match` each. Since an earlier commit wrote `&mut **e` in both, the two are equal arm for arm, which the `same_match_twice` lint reports. The mapping is now a method of `EntriesOption`, next to `entries_mut`, which panics on `Err`. Both callers apply `?` to it. The result borrows from `&mut self`, which is `'static` for the value that `read_directory` returns. Nothing changes in the compiled code: the method is inlined into both callers. #### 53 and 54. Two fixes for tests of `main`'s, which the merge of `main` takes out again `test/aot/engine/ir-6.test.ts` and `test/aot/build-scaling/build-scaling-2.test.ts` failed without this PR too. Commit 53 brought in the change that #47074 makes to one fixture, and commit 54 the two files of #47329, byte for byte. #47316 has since rewritten all three files on `main`. The fixture no longer has the eleven types that commit 53 relaxed, and the shape of commit 54 no longer has a recorded growth: it went from 17,206 to 4,416 million instructions. Two of the three files conflicted. The merge takes `main`'s version of all three. #### 56. resolver: `read_directory` and its siblings return `&'static EntriesOption` `FileSystem::read_directory`, `RealFS::read_directory`, `read_directory_with_iterator` and `read_directory_error` returned `&'static mut EntriesOption`: a slot of the shared directory cache, or the thread's scratch slot. They are safe functions, so two calls for one directory gave safe code two `&mut` to one slot. That is so on `main` too, for all but `RealFS::read_directory`, whose result change 20 widened to what its callee returns. `entries_at_locked` handed out a `&mut` to the same slots. No caller writes through the result: - The package manager reads `data`, `dir` and `fd` and calls `has_comptime_query`. - The test scanner reads the error or walks `data`. - `DirEntryAccessor::openat` and `load_as_file` made a shared reference of it at once. - Both callers of `entries_at_locked` call `as_entries`. So they all return a shared reference, `PackageManager::root_dir` is a `&'static DirEntry`, and the `try_entries_mut` of commit 52 is `try_entries(&self)`. What stays as it is on `main`: a read with a newer generation rewrites a slot's `DirEntry` in place under `entries_mutex`, and the next miss on a thread overwrites its scratch slot, while an earlier result may still be held. Readers of `data` take the mutex, and the doc of `read_directory` says to read an `Err` before the next call. ### How did you verify your code works? - **Static checks on the first 52 commits, which hold all the code:** `cargo fmt --check`, `cargo clippy --workspace` (0 warnings), the source lints and `cargo check --workspace --release` on Linux, and `cargo check --workspace --release` for `x86_64-pc-windows-msvc` and `aarch64-apple-darwin`, because seven changes touch code that only those targets compile. - **Assembly:** the comparison above. - **230 test files**, on a debug build with ASAN of these commits and on a debug build of the base: all 114 files of `test/cli/install/`, and the files for the rest: TOML and XML, source maps, the shell's brace expansion, `test/js/bun/resolve/`, the bundler's resolver tests, `bun build --compile`, HTTP/3 and TLS, `console.log` and `Bun.inspect`, auto-install and `bunfig.toml`. - 220 files pass completely, 17,876 tests. - 10 files fail, in exactly the same tests on both builds: | File | Tests that fail | Why | |---|---|---| | `bun-run.test.ts` | 28 | They run `bun-debug` by name, and it was not on my `PATH`. | | `bun-install-git-deps.test.ts` | 19 | My `git` refuses to make their tags ("no tag message?"). | | `bun-install.test.ts` | 4 | Git URLs. | | `bun-install-lifecycle-scripts.test.ts` | 2 | "does not use 100% cpu", on a machine that ran other tests at the same time. | | `bundler_compile.test.ts` | 7 | Packages that I did not have. | | `bun-build-compile.test.ts` | 3 | Their time limit of 60 seconds. | | `run-crash-handler.test.ts` | 1 | It expects a frame name that a debug build prints differently. | | `resolve.test.ts` | 1 | `tsyringe`, which I did not have. | | `stringWidth.test.ts` | 1 | `string-width`, which I did not have. | | `inspect-error-leak.test.ts` | 1 | Its time limit of 10 seconds: 115 seconds here and 116 on the base, and +359 MB on both. | - The 13 source-map files and `brace.test.ts` ran after my last edit to change 24, the way a `WindowReader` gets its stream. The other files ran before it. - **After the tests had run** I removed six `#[inline]` attributes that an earlier form of changes 3 and 9 had added, and took one edit out of change 14, so that the crash handler calls `libc::abort()` as it does on `main`. The static checks ran again after that, and so did the assembly comparison of `bun_crash_handler`, with the numbers above. - **`bun.lockb`:** a build of the base, a build of the first 52 commits and Bun 1.4.4 write the same 10,218 bytes for one project, which has ranges with `||`, a hyphen, `*`, `1.x` and prereleases, an alias, a dist-tag, two catalogs, two workspaces, an override, and peer, optional, dev and trusted dependencies. Each of the three reads what each of the three wrote: `bun bun.lockb` prints the same text and the same hash, `bun install --frozen-lockfile` passes and leaves the file as it is, `bun pm ls --all` prints the same tree, and `bun add` turns the file into the same 10,554 bytes. The ten `.lockb` files in the repository print the same text on all three. - **Commits 51 and 52 came after that.** With them I ran all the static checks again and compared the assembly of `bun_parsers`, `bun_resolver` and `bun_install` again: 52 changes no instruction, and the numbers above for `bun_parsers` include 51. On a debug build of these 52 commits, `xml.test.ts`, `xml-test-suite.test.ts`, four auto-install files, `bun-remove.test.ts`, `bun-link.test.ts` and `bun-pm.test.ts` pass completely, 2,315 tests. `bun-add.test.ts` and `run-autoinstall.test.ts` did not finish in the 50 seconds that I gave each file. - **The merge of `main` (3bf1a0646) came last.** Everything above is from before it. - The only conflicts were in two of the three test files of commits 53 and 54. 16 files under `src/` that both sides changed merged by themselves. - The 2,496 lines that this PR adds and removes under `src/` are the same before and after the merge, in the same order, and so are the counts of `unsafe` against `main`. - On the merged tree, `cargo fmt --check`, the source lints and `cargo clippy --workspace` (0 warnings) pass, and a debug build with ASAN and a release build link and run. - On the debug build I ran 30 test files for the code that both sides changed and for the parsers and the package manager: `import.meta.glob`, import queries, `test/js/bun/resolve/`, the bundler's resolver cache, source maps of compiled executables, the crash handler, `bun:ffi`, TOML, XML, `bun pm`, `bun link`, `bun remove` and `bun.lockb`. 29 pass completely, 3,997 tests. In `resolve.test.ts` one test fails, the one that needs `tsyringe`, as in the table above. - **Commit 56** came after the merge. With it, `cargo fmt --check`, the source lints, `cargo clippy --workspace` (0 warnings) and `cargo check --workspace --release` pass on Linux, and so does `cargo check --workspace --release` for `x86_64-pc-windows-msvc` and `aarch64-apple-darwin`. The release binaries with and without it have the same bytes in `.text` (105,908,048 bytes), `.rodata`, `.data`, `.data.rel.ro` and `.eh_frame`. --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
…rust-17fiip One conflict, in src/runtime/image/codec_jpeg.rs: main made the declaration of tj3GetScalingFactors safe and took the unsafe block off its call (#47944). This branch has neither any more, so the file stays as it is here.
…a coefficient around 16 bits A first scan with Al = 13 can leave a coefficient of -32768. A refinement scan that corrects it subtracts 4096, and the sum wraps around to 28672, as it does in libjpeg-turbo. With saturating_add in the place of wrapping_add, in decode/phuff.rs or in decode/arith.rs, every test passed. A Huffman-coded file of 164 bytes and an arithmetic-coded one of 120 bytes have that coefficient and nothing else. What is expected of them comes from Bun 1.4.4, and this branch gives the same. Bun built with either change fails the golden test on its new file and on no other file. Four more changes of the kind were tried and are caught already: saturating_add for the DC prediction of a baseline scan, wrapping_add and saturating_add for the checked sum of a progressive one, and a height of 0 let through.
…o limits, running out of memory With the call of codecs::guard in codec_jpeg.rs deleted, every test passed: metadata() has a check of its own, and no test decoded a JPEG of more pixels than maxPixels. Three more changes to that file passed as well: the limit of 2^31 - 1 pixels looked at before maxPixels, that limit halved, and a failed allocation reported as a failed decode. - 16 x 4 and 4 x 16 are decoded with maxPixels: 64 and refused with 63. - A JPEG over both limits is over maxPixels. The sizes over 2^31 - 1 are now 65500 x 32787 both ways as well as 46341 x 46341. - 65500 x 32786, both ways, is 647 pixels under that limit and takes 8.6 GB of RGBA. A child that may not have them, by RLIMIT_AS or by the sanitizer's own limit, rejects with ERR_OUT_OF_MEMORY. Bun 1.4.4 aborts there. Linux only. Bun starts with 0.5 GB of address space; the child has 8. Bun built with each of the four changes fails these tests, and so it does with width times width or height times height in the place of the product, in either check.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Removes the dependency on libjpeg-turbo.
Bun.Imagedecodes and encodes JPEG withsrc/jpeg(bun_jpeg, new,#[forbid(unsafe_code)]), a translation to Rust of the parts of libjpeg-turbo 3.2.0 that Bun reached through the TurboJPEG API. Five kernels that work on a row of samples (color conversion both ways, downsampling, upsampling) are C++ with Highway, as the repository's other SIMD is:src/jsc/bindings/highway_jpeg.cpp, behind safe wrappers insrc/highway/jpeg.rs, compiled for SSSE3, AVX2 and NEON. An AVX-512 variant made a whole encode or decode no faster (35.4 against 34.4 ms, 26.4 against 26.8 ms on Cascade Lake) and is gone.src/runtime/image/codec_jpeg.rskeeps its policy (the pixel limit, which M/8 scale a resize target picks, CMYK conversion, when the ICC profile is kept) and loses the FFI, 330 lines of it.src/jpegcarries libjpeg-turbo'sLICENSE.mdandREADME.ijgunaltered.NOTICE.mdlists the 63 files it derives from with the attribution of each header word for word, and says up to which upstream commit the history was checked for fixes.The goal is no observable change, quirks included: the same streams decode, to the same pixels, the same streams are rejected, and the same pixels encode to the same bytes. Some of what that took:
idct.rsdoes what those instructions do, lane by lane. The other sizes ran as C with a 64-bitlong.FF FF 00.huff.rshas both routines, and reads ahead exactly as far as they did, because that decides which one sees the bytes.tj3GetScalingFactorsdid.Changes in behaviour
TJPARAM_SCANLIMIT, which Bun never set. 500 is whatTJFLAG_LIMITSCANSstood for. In a corpus of 286,000 files from 47 encoders and tools the most is 18validate_scriptaccepts passes (6,000 random ones tried, and 33,439 files of 1 to 100 scans thatcjpeg -scanswrote from such scripts). Of 554,250 files from 193 origins none that an encoder wrote is refused (56 origins), nor is any of 27,405 files made for this with four encoders that owe nothing to libjpeg (the crates jpeg-encoder and image, Go'simage/jpeg, jpeg-js). Apart from libjpeg-turbo's own fuzzing seeds and files damaged on purpose, 7 distinct files that used to decode with warnings are refused now, all of them a fuzzer's or a crash test's (among them Blink's182.jpgandsubtract-with-overflow.jpgof jpeg-decoder). Models of the rule written independently of the code, one of them 15 lines of C in a copy of libjpeg-turbo, agree with it on 31 million scripts of scansERR_OUT_OF_MEMORYtj3GetICCProfilefor no profile and ignored the result oftj3SetICCProfile). 12 files with huge dimensions, 5 operations, 8 limits on the address space: Bun 1.4.4 aborts in 242 of the 480 cases, this branch in none, and the same 220 succeed. Outside the codec (resize buffer, PNG, WebP) a failed allocation still aborts, as before. One such case is a test now, on Linux: 65500 x 32786, 8.6 GB of RGBA, in a child that may have 8 GB of address spacesrc/jpegparses the header once. Bun 1.4.4 runs out of time in the new test, three times out of threelong, which has 32 bits on Windows. There is one implementation now, and it follows the 64-bit one. Of the same 31,560 results, 92 differJSIMD_FORCENONE,JSIMD_FORCESSE2andJSIMD_NOHUFFENCin the environment do nothing any moreCarried over on purpose, and open to a decision:
maxPixelsand the two rules on scans.maxPixelsis.jddiffct.c), not the standard's:djpegdecodes 56 of 84 conforming files of that kind differently from their source.cjpegnever writes a subsampled lossless file.What a hostile file can cost
An earlier version of this description called 15.4 s the most expensive file at the default
maxPixels. That was wrong by three orders of magnitude, for libjpeg-turbo as for this code, which take about the same time on all of these. An arithmetic decoder goes on deciding after its data has run out, as the standard wants, and 150 bytes can train its statistics so that every later block costs 2,100 decisions.A coefficient has a first scan and up to 13 that add a bit, so a script of 500 scans can still keep to the rule, but each bit is now decoded once: more than half of that time is one dense pass. All of it is spent before any output, so it costs the same for a thumbnail. What is left is the arithmetic decoder going on after the end of its data, and I have no fix for it that I would defend. A limit on the zero bytes made up after the end of the data changes the pixels of files that
cjpeg -arithmeticitself writes: it drops trailing zero bytes, 131,055 of them for a 4096x4096 ramp. Candidates: a budget of decisions per block once the data has ended (a prototype passes all 390 legitimate arithmetic files at hand and turns 174 s into 9 s), a lower pixel limit for arithmetic files, a lower limit on scans (100 would refuse one more known file, a stress test of Chromium's) or a limit on blocks visited over all scans, a deadline inDecodeOptions. Peak memory is the same as libjpeg-turbo's: 3.08 GB for 135 bytes at the defaultmaxPixels.Performance
The codecs alone, 3000x2000 synthetic photograph, one thread, taking turns. Column 1: AMD EPYC (Zen 4), both compiled as Bun's release build does (
-O3, Nehalem, ThinLTO across both languages), at 63485ca. Column 2: Intel Xeon (Cascade Lake), the harness's build (no LTO, libjpeg-turbo's C at-O2, which flatters this code a little), machine not idle, at 0f31846.A wider matrix on a third machine, which was busy (the harness's build for Nehalem, synthetic pictures, at b01c1cf). 432 encodes, 6 kinds of picture by 6 sizes by 6 qualities in both modes: sequential 1.18x in the sum, median 1.25x, at most 1.58x; progressive 0.83x in the sum. A sequential encode of 16 megapixels or more takes about 0.6 ns a pixel longer whatever the picture, while libjpeg-turbo's own cost goes from 1.6 ns (flat) to 4.6 ns (noise), so the gap lies before the entropy coder. How a decode at 8/8 compares depends on how much entropy-coded data the picture has. 240 files of 3072x2048 from
cjpeg(4 kinds of picture, 10 qualities, 4:4:4, 4:2:2 and 4:2:0, standard or optimised tables): under 2 bits a pixel, where a photograph at q90 or below lies, 1.11x to 1.16x in the sum, and this code is the slower side on 118 of the 120 files; from 2 bits a pixel up, 0.93x to 0.97x. In a process that is warm, a photo-like picture at q85 takes 1.18x at 4:2:0 (15.8 against 13.4 ms), 1.17x at 4:2:2 and 1.29x at 4:4:4. By a sampling profile of a build for Haswell the difference is the inverse DCT, 6.4 against 3.1 ms at 4:2:0, while entropy decoding is level or faster. At the reduced scales, which aresize()to less than the source takes, the files under 2 bits a pixel take 0.82x at 7/8, 0.89x at 6/8, 0.99x at 5/8, 1.18x at 4/8, 1.08x at 3/8, 1.15x at 2/8 and 1.02x at 1/8 (libjpeg-turbo has SIMD code for the 4x4 and the 2x2 inverse DCT), and the files above 0.80x to 0.90x. With the standard Huffman tables at q100, noise and text take 1.26x to 1.34x, and there the difference is in entropy decoding; with optimised tables the same pictures take 0.84x to 0.93x. Progressive, gray and 1x2 files take 0.90x to 1.02x, arithmetic ones 0.98x to 1.12x, files with a restart marker every MCU row 1.07x to 1.14x. (An earlier version of this paragraph gave only a sum over 180 files of all modes, 0.91x to 0.97x. Noise, text and arithmetic files dominate that sum, and it hid the first case, which is the common one.) A 1x1 picture decodes in 2.5 against 6.2 microseconds. Files of 4 to 64 MB, blown up with filler of 149 kinds, take time in proportion to their size on both sides.bench/image/bench-resize.mjson the Zen 4 machine, Bun 1.4.3 against a release build: 1.02x to 1.16x, one case (4K JPEG to 1080p JPEG) 1.19x to 1.28x.On files that nobody made to be slow, a sample of 4,900 from 193 origins, those on which libjpeg-turbo takes a million cycles or more (loaded machine, at b01c1cf): at 8/8 the median is 1.03x in cycles and 1.01x in CPU time, the 99th percentile 1.41x, the most 1.57x; at 3/8 0.93x and 0.92x, 1.34x, 1.43x. Peak memory: median 1.00x for small and for large files, the most 1.32x (65,500 pixels wide and 9 high). Encoding a picture 65,500 wide takes 3 MB more. Files with no data at all take up to 2.2x at 2/8. Very narrow and tall subsampled images take up to 2x, a few milliseconds.
The five Highway kernels take as long as libjpeg-turbo's assembly or less. What is left of the gap is the kernels that work on a block and are still loops for the vectorizer: alone, the 8x8 inverse DCT takes 163 cycles against 86 and the 4x4 112 against 53, and by one profile a quarter to nearly a half of a sequential encode goes into the two transpositions of the forward DCT. Writing those with Highway too is the next step, and not in this PR. Speed is not measured on arm64 or Windows.
[profile.dev.package.bun_jpeg] opt-level = 3keeps debug builds usable: atopt-level0 the crate takes 20 to 60 times as long.Where the stack lies can matter to the sequential encoder, though not in the build of Bun that I could measure. The forward DCT hands a block from stage to stage through four buffers of 128 bytes on the stack, and nothing in the source aligns them. In a build of the crate alone (Zen 4) the compiler aligns two of them by 8 only. At the 16 of the 256 places of the stack within a page at which one of those two straddles a page, a flat 4000x3000 picture takes 1.12x to 1.17x as long, because a 16-byte access is split between pages; an independent measurement with another toolchain found 1.25x to 1.4x at 8 places and 1.1x to 1.25x at 16, which are exactly the places at which one of the three buffers that build aligns by 8 straddles a page. The worst of them is used six times for a block. With all four aligned by 16 that band is gone, but the wrapper type I tried it with cost more than it saved (30.7 ms against 26.0 ms), so it is no fix as it stands. In a release build of Bun on the same machine the compiler aligns all four by 16. Encoding runs on worker threads, so there the place is the same in every run of one build, and moving it through 239 of the 256 places changes the time by at most 2% (flat) and 4% (like a photograph). I could not look at the binaries that CI builds. Kernels that keep the block in registers would end this.
Every release binary gets smaller, by 131 to 328 KB: Linux x64 -308 KB, Linux aarch64 -256 KB, macOS x64 -306 KB, macOS arm64 -194 KB, Windows x64 -284 KB, Windows arm64 -182 KB (#136360 against the build of
mainat the commit that the branch has merged in).How did you verify your code works?
Against libjpeg-turbo itself.
misctools/jpegdiff(temporary) linksbun_jpegnext to libjpeg-turbo 3.2.0 built with Bun's patches and flags, SIMD on, and replays whatcodec_jpeg.rsdid at the commit before this branch. Decoding compares pixels, dimensions, ICC profile, and whether and at which stage it fails, at all 8 scales. A stream thatbun_jpegrefuses for a redundant scan and libjpeg-turbo decodes is counted and not compared: about 2% of the generated streams (see "Not covered"). Encoding compares bytes.bun_jpegis compiled with overflow checks and debug assertions. Three independent fuzzing efforts build on it, with generators, harnesses and corpora of their own. Over the life of the branch, with no difference since the first 22 million mutations found theFF FF 00case:cjpegof every IJG release from 4 to 10 and of libjpeg-turbo 1.0 to 3.2,jpegtranof 17 releases, encoders that share no code with IJG (the ISO reference software, PVRG, ffmpeg, Guetzli, Go, Rust, Java, jpeg-js, stb, jpge), 79 other projects' test suites, JPEG streams out of PDFs, DICOM files and thumbnails: 30 million comparisons through copies of the old and the new gluedecode_mcuagainst an instrumented libjpeg-turbo: 109 million streamsBun.Image, Bun 1.4.4 against builds of this branch: more than 35 million cases, among them 140,400 files that libjpeg-turbo's API wrote at every width from 1 to 300 in 36 configurations and 117,000 encodes at the same sizes, 23 ways to pass the input and 18 to take the output, among themnew Response(image), which runs on the JS thread; inputs flush against unmapped pages; pools of 2 to 1,024 threads; images of 2.15 gigapixels; inputs of 6 GiB; 30 scenarios of 300,000 operations for leaksAgainst the history. For each part of the code, every upstream commit that touched the files it derives from was read and classified, on all branches and IJG's releases: between 61 and 467 commits for each of 8 parts, up to the 57 after 3.2.0. No fix is missing. So were all 399 entries of
ChangeLog.md, IJG'schange.logfrom 6b to 10, 51 CVEs against libjpeg-turbo, IJG, mozjpeg and the copies in Chromium, Android, DCMTK, GDCM and the distributions (11 carried over, 17 cannot arise, 23 in code that was not ported; for CVE-2016-6702, CVE-2020-0207, CVE-2017-8826 and CVE-2018-5252 I could not find out what the flaw was), and 136 CVE, GHSA and RUSTSEC ids of other JPEG decoders in 48 classes of mistake, each with crafted files (about 100,000 files at 8 scales for the two). A second table by one of the fuzzing efforts has 702 rows in 44 groups and 50 million comparisons. Its crafted files were tried on 37 old releases of libjpeg-turbo and IJG under ASAN, UBSAN and MSAN: of the 54 bugs whose code has a counterpart here, 39 show themselves there and not in 3.2.0, and the other 15 are of kinds that a sanitizer cannot show (cost and limits, assembly, 32 bits) or, for one, were not tried. libjpeg-turbo's own reproducers (277) and AFL test cases (2,996) give the same results as 3.2.0.A review of the whole PR, part by part, each line against its counterpart in C or assembly. No difference from libjpeg-turbo, panic, hang or memory error in the code as it was. What it did find, all fixed here:
maxPixelsraised), a symbol that occurs more than 10^9 times in a progressive scan got no Huffman code, and the encoder returned a corrupt file where libjpeg-turbo fails. (libjpeg-turbo itself indexes with an uninitialisedbit_pos[0]there,jchuff.c, also on itsmain.)NOTICE.mdleft out Guido Vollbeding, the notice ofjsimdext.incand seven headers.LICENSE.mdlacked the sentence the IJG license asks for.highway_jpeg.cpphas a list of Highway targets of its own, which is sound only whilehwy::ChosenTarget::GetIndex()is inlined. Without optimization the linker would merge it with other files' copies. It now refuses to compile that way.src/highway/jpeg.rs, a debug build with ASAN passed every image test with any of 10 changes that take away a check that keeps a kernel inside its rows, and with any of 10 that make a wrapper leave to the scalar code what a kernel should do: the kernel tests call the C++ functions themselves, and the codec gives the wrappers room enough and, but forto_ycc, rows of one length. A second hook now calls the wrappers with rows of any lengths, and all 20 fail there, on a debug build with ASAN and on a release build. So do 35 more that were tried later, among them every length and every bound off by one. A last sweep over the crate andcodec_jpeg.rs, 886 changes of kinds that none of the earlier ones made (one side of a||or&&dropped, a loop or ananynarrowed to its first or last item or to all but it, areturn Errremoved, an alternative of a pattern dropped), found 14 more in the decoder. 11 more files close them: Bun built with each of the 14 fails the golden test on its new file and on no other. Of 77 further changes that a larger set of inputs had caught only late, the tests in the repository catch 60 (all 39 in the encoder by the 466 pictures, and 21 in the decoder, 16 of those by nothing but the damaged copies of the golden files), 15 change only which error a file fails with, and 2 more files close the last 2, shown in the same way. Of 12 more that change behaviour and had been hard to catch elsewhere, 9 fail the tests in the repository, 1 cannot show throughBun.Image, which asks for FF D8 FF before it calls the codec, and 2 more files close the other 2:saturating_addforwrapping_addwhere a refinement scan corrects a coefficient of -32768. Last, 72 changes tocodec_jpeg.rsalone, in a release build of Bun: 51 failtest/js/bun/image/, 16 change nothing that can be observed, 1 needs a small allocation to fail, and 4 passed. One of the 4 deletes themaxPixelscheck in front of a decode:metadata()has a check of its own, and no test decoded a JPEG of more pixels thanmaxPixels. The others look at the limit of 2^31 - 1 pixels beforemaxPixels, halve that limit, and report a failed allocation as a failed decode. Tests inimage-adversarial.test.tsclose them: Bun built with each of the 4 fails them, and so it does with width times width or height times height for the product in either check.Tests in the repository.
test/js/bun/image/jpeg-golden: 870 files, each at all 8 scales throughresize(), 3 damaged copies of each at 2 scales, pixels and ICC profile, and 466 generated pictures encoded in both modes, at every quality. The expected results were recorded with Bun 1.4.4 (1.4.3 records the same), which used libjpeg-turbo, but for the 20 that the rule on redundant scans changes.README.mdthere lists those and says where the files come from.image-kernels.test.ts: each Highway kernel, through a hook inbun:internal-for-testing, against libjpeg's formulas in JavaScript, in memory that ASAN watches (a test proves that it does). And each wrapper insrc/highway/jpeg.rs, through a hook in safe Rust (src/runtime/image/jpeg_rows_testing.rs), with rows of different lengths and too little room, against the kernel on the shortest length.simd-baseline.test.ts, which CI also runs under an emulated Nehalem: pinned bytes and pixels, a 4:2:2 and a YCCK file.image-adversarial.test.ts,image.test.ts: both rules on scans, DC predictions at the ends of 32 bits, 65500 and 65501, 2^31 - 1 pixels, ICC profiles around the size of a segment, photographs, a JPEG rewritten while it is decoded, as a whole and in its height alone.mainat 2596ac5 merged in:test/js/bun/image/andsimd-baseline.test.ts515 pass, 8 skip, with the tests of the PNG codec. A release build of the same commit: the same tests 514 pass, 9 skip, andtest/internal/,test/aot/builtins/(one of its files by itself, as in CI),test/aot/engine/,test/aot/engine-tests.test.tsandbun-types.test.tspass. The harness on b01c1cf, since when nothing in the codec but a comment has changed: the 1,037,127 files that the review crafted at 8 scales, 6 million mutations, 200,000 encodes, no difference.test/internal/source-lints/,bun-types.test.ts,cargo clippy -p bun_jpeg -p bun_highway -p bun_runtimepass.bun run rust:check-allpassed at 5093d08 and was not repeated.Not covered: real arm64 and Windows machines other than by CI. No test in the repository makes an allocation fail or reaches the three refusals of the encoder's table generation (
bun_jpeglinks the C++ kernels, so it has nocargo test); the harness does both. Eight small changes to what that table generation does with codes of 31 to 33 bits show only in pictures of 10 to 26 megapixels, too large for a test; the function was compared withjpeg_gen_optimal_tableitself on 889 million tables of frequencies, without a difference. The independent fuzzing uses rustc 1.98.1 with LLVM 21, not Bun's pinned nightly; my own runs use the pinned toolchain.misctools/jpegdifftakes every refusal for a redundant scan as intended, without checking that the stream has such a scan, and it does not check that a stream which decodes has none. One of the three fuzzing efforts never went by that: its reference is a copy of libjpeg-turbo with the rule patched in, and a refusal on one side only is a mismatch. The other two did until 30 October, so their runs up to then do not show that the rule fires when it should and only then. They now hold every stream against a model that reads the stream's bytes, both ways, and have found no mismatch. In the harness: 15 million generated streams with 334,787 refusals, 3 million mutations with 47,033, and the regression sets on 7 builds. ThroughBun.Image: 86 million new cases against Bun 1.4.4 with 14.5 million refusals, 143 million cases of the exhaustive suites (every change of one byte), and 130 million inputs of earlier runs made again, of which the model calls 5.6 million redundant and none decodes. Both find wrong versions of the rule that were put in on purpose, 3 and 9 of them. Runs whose inputs cannot be made again have not been repeated, andmisctools/jpegdiffitself does not have the model.The tests in the repository had gaps of their own here. Of 22 wrong versions of the rule, Bun built with each, 4 passed every image test: one that lets a second first scan of a coefficient through if it goes down to a lower bit, one that looks at the first coefficient of a band only, and two that look at the first or the last component of a scan only. 6f83dfd adds the cases that tell them apart. All 22 fail now, each of the four on new cases only, and Bun 1.4.4 fails every new case.
Also in this PR
test/bundler/compile-trim-by-use.test.tslooked for libjpeg-turbo's messages in a trimmed executable. None ofbun_jpeg's texts is linked, so it looks for two of its tables.exif.rssearches for the next marker withindex_of_char_raw: the other function asserts, in a debug build, that the bytes hold still.To do
misctools/jpegdiff(it is worth keeping until the block kernels are done)bugs/in libjpeg-turbo/seed-corpora, which has no licenseFound on the way, the same in Bun 1.4.4 and not touched here:
Bun.Imagereads freed memory if JavaScript transfers the input's buffer or grows the WebAssembly memory it is in during a decode, with every format;new Response(image)ignores the slice of a slicedBun.file; nothing can stop a decode once it runs.Generated by Claude Code